Skip to content

perf(agc): append shader keys directly to the draw resource key - #2

Open
JJenkx wants to merge 1 commit into
mainfrom
perf/draw-resource-key-append
Open

JJenkx wants to merge 1 commit into
mainfrom
perf/draw-resource-key-append

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

What

DrawResourceKey builds a temporary content-key vector for every shader and copies it into the combined draw key. Append the existing visitor’s words directly to the combined vector and fill the same stage-length prefix, removing those temporary vectors.

The refresh retains main’s exact content-key capacity calculation, overflow checks, current descriptor masks, and key-difference labeling. The combined draw reservation has checked arithmetic. Added coverage verifies that append operations preserve a pre-existing prefix and earlier stages across all 24 existing configurations, including descriptor counts through 16,384.

Tested

One linear commit on main a453108cf77. Release build of graphics, draw-cache-reuse, and sampled-cache-budget test executables passes. The CPU content-key cases pass.

Graphics validation, draw-cache reuse with GPL on/off, storage admission, oversized-storage admission, and storage clear pass on both RADV/Radeon and lavapipe: 12 test invocations passed, no whole-test skips. The graphics tests report that heap-mirror subchecks were not exercised because a write-watched guest arena was unavailable. Conventions and whitespace checks pass.

No matched title benchmark or Windows/full-suite validation has been run for this refreshed revision. Earlier profile numbers do not establish a gain on this base; no gameplay FPS improvement is claimed. This remains a fork contribution for testing before an upstream performance submission.

Checklist

  • One topic; generic key construction
  • No code comments, assets, or new dependencies
  • Current key fields, ordering, and existing checked capacity behavior retained
  • Depends on: none
  • Unblocks: none
  • GPUs tested: AMD Radeon RX 6650 XT, RADV Mesa 26.2.4, Linux; lavapipe Mesa 26.2.4 on the same Linux host
  • AI-assisted: yes

@github-actions github-actions Bot added the open PR limit Closed over the open PR limit: 1 without merged PRs, 20 with at least 1, none from 20 label Oct 12, 2026
@github-actions

Copy link
Copy Markdown

Thank you for your contribution! Authors with no pull requests merged into main can have 1 open pull request at a time, authors with at least 1 merged can have 20, and from 20 merged there is no limit. This PR has been automatically closed and labeled "open PR limit" because that limit was reached. This is not a judgment of the quality of your changes: reopen it once one of your open pull requests is merged or closed.

@github-actions github-actions Bot closed this Oct 12, 2026
@JJenkx JJenkx reopened this Oct 12, 2026
@JJenkx JJenkx removed the open PR limit Closed over the open PR limit: 1 without merged PRs, 20 with at least 1, none from 20 label Oct 12, 2026
@github-actions github-actions Bot added agc libSceAgc* needs rebase Conflicts with main, rebase needed labels Oct 13, 2026
Avoid allocating and copying a temporary content-key vector for every shader of every draw. Reserve the combined key up front and append the same visited words with the existing per-stage length prefix. Full integration build, eight selected Radeon checks, software draw-cache reuse and conventions pass. RollerCoaster park profiling motivated this change; performance improvement has not yet been measured.
@JJenkx
JJenkx force-pushed the perf/draw-resource-key-append branch from f0e8fc7 to cac3a95 Compare October 14, 2026 15:42
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
JJenkx pushed a commit that referenced this pull request Oct 15, 2026
The pr-template check only accepted "Depends on: #N" or "none" on the same
line, so a checklist answering it as a bulleted list failed CI although
pr_overlap reads exactly that form (test_pr_overlap_dependencies covers
"Depends on:
- #2
- #3"). Allow one list marker before the first answer;
an empty answer is still reported.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zyXkUzTZGh3nBwNMtrLaW
JJenkx pushed a commit that referenced this pull request Oct 16, 2026
…ll or crash

A GPU hang or a crash in a title leaves no record of what the queues
were last given: APS5_TRACE_GPU prints every submission (too much to
leave on), and the WAIT_REG_MEM timeout message shows only the packet
before the wait on the waiting queue.

Each queue now keeps a ring of its last 16 submissions
(APS5_PM4_RING=<n>, 0 to 4096, 0 disables): the TraceMs time in tenths
of a millisecond, serial, size, guest address and a copy of the first
512 dwords after indirect buffers are inlined, recorded under the
driver lock next to the serial, REWIND tails included. The ring of a
queue and the failure report registration are set up at the top of
Submit, before any output is reserved, so recording under the lock
does not allocate. Disabled, Submit only tests a static flag; enabled,
it costs one bounded copy per submission.

DumpSubmissionRings decodes the kept dwords with Pm4::PacketWords and
the new Pm4::KnownName, which returns the decoder's names as views of
static strings (empty for an unknown opcode, printed as UNKNOWN_0x%02x);
Pm4::Name now builds its std::string from it. Every line is formatted
with integer and string conversions only into a 512-byte stack buffer
and written with one WriteFile / write(2) on the stderr handle, so the
dump does not allocate from the C runtime heap, lines from other
threads do not interleave and the CRT stream lock is not taken (the
rule of boykopovar#3871). The header gives the number of queues and the ring
size, and each queue line the submissions recorded and kept.

libc gets a small registry of failure reports
(LibcRegisterFailureReport_nid_no_patch, Unregister, Run); Run runs
the registered reports the first time it is called and returns at once
after that. The Windows crash report runs it after the thread dump of
an unhandled exception, after SIGABRT (which also covers the
APS5_HANG_DUMP_SECS timed dump, since it aborts) and after an invalid
CRT parameter, so an abort after a reported exception does not print
the rings twice. The driver registers its dump at the first submission
and unregisters at static destruction; the dump there waits a bounded
time for each ring lock. What the crash-path dump cannot do (a ring
held by the crashed thread, rings corrupted by the crash, Linux) is
recorded in technical debt.

APS5_STALL_WATCHDOG_SECS=<n> (0 to 86400, 0 disables) makes a waiter
that has not progressed for n seconds dump the rings once per wait: a
WAIT_REG_MEM (progress as for APS5_GPU_WAIT_TIMEOUT_MS: another packet
runs or completes), a REWIND waiting for the title, and a compute
submission held behind queue 0. The last one prints under the driver
lock, so submissions and the other queues wait for it (USAGE.md says
so). Both variables are read at the top of Submit, so an invalid value
of either throws at the first submission and never later inside a GPU
worker. The order wait behind queue 0 releases its order hold on scope
exit, so a throwing dump cannot leave it counted.

No packet semantics change; packet sizes and names are those of the
existing decoder (PM4 type-3 header layout as in Mesa
src/amd/common/sid.h). The queue bound of Submit and of the rings is
one constant, SubmissionQueueEnd.

Tests: agc_driver_pm4_tests gets ten modes, each its own ctest:
agc_driver_pm4_ring (APS5_PM4_RING=4, six submissions on queue 0 and
two on 0x20: the dump holds #3..#6 and #1..#2 in order, nothing
older), agc_driver_pm4_ring_empty (one line), _ring_disabled (one
line), _ring_invalid and agc_driver_pm4_stall_watchdog_invalid
(APS5_STALL_WATCHDOG_SECS=1x: the submission throws and is not
executed), _ring_report (LibcRunFailureReports_nid_no_patch dumps the
rings), _ring_report_heap (Windows: with ucrtbase.dll's HeapAlloc and
HeapReAlloc imports counting calls from the reporting thread, the
failure report of a submission holding INDEX_BUFFER_SIZE names it and
allocates nothing, while Pm4::Name of the same header is seen
allocating), and with APS5_STALL_WATCHDOG_SECS=1 a WAIT_REG_MEM
(agc_driver_pm4_stall_watchdog), a REWIND (_stall_watchdog_rewind) and
a compute submission held behind a queue 0 rendering wait
(_stall_watchdog_order), each held for about 2.5 s, dump once and
complete when released. The PM4 catalog test checks KnownName against
every opcode and custom name. windows_backtrace_failure_report checks
that abort() runs the registered reports once and not an unregistered
one; windows_backtrace_failure_report_once that abort() does not run
them again after an earlier run. With the dump decoding through
Pm4::Name, agc_driver_pm4_ring_report_heap fails with "the failure
report allocated from the C runtime heap"; with Run resetting its
flag, windows_backtrace_failure_report_once prints the report twice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@svetlana-ilyankova

Copy link
Copy Markdown

null,

AI Response is:
Hello! It looks like you provided the details of a pull request (JJenkx/AnyPS5#2) along with additional context regarding an automated GitHub action that closed it due to open pull request limits for new contributors.

How can I help you with this today? For example, I can:

  • Help you summarize or rewrite the pull request description.
  • Draft a response message for the PR thread.
  • Help you understand repository contribution guidelines.

Let me know what you need!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants